-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix memory leak in IIS StartupHook by properly disposing PhysicalFileProvider #63322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…Provider Co-authored-by: danmoseley <[email protected]>
|
@BrennanConroy any opinion on whether ErrorPageModelBuilder.CreateErrorPageModel() ought to do the dispose here? Indeed it is not used after that call so the fix as is is reasonable. |
|
The |
|
OK. Are you comfortable with the dispose in this particular context? ie this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a potential memory leak in the IIS StartupHook class by ensuring proper disposal of a PhysicalFileProvider instance used in unhandled exception handling.
- Wrapped
PhysicalFileProvidercreation in ausingstatement to ensure automatic disposal - Replaced inline instantiation with a properly scoped variable reference
- Applied existing codebase patterns for resource management with file providers
| var iisConfigData = NativeMethods.HttpGetApplicationProperties(); | ||
| var contentRoot = iisConfigData.pwzFullApplicationPath.TrimEnd(Path.DirectorySeparatorChar); | ||
|
|
||
| using var fileProvider = new PhysicalFileProvider(contentRoot); |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a blank line after the using statement declaration to improve readability and follow the formatting guidelines that specify inserting newlines before code blocks.
The SVACE static analyzer identified a potential handle leak in the IIS StartupHook class where a
PhysicalFileProviderwas created but not properly disposed.Problem
In
src/Servers/IIS/IIS/src/StartupHook.cs, the unhandled exception handler creates aPhysicalFileProviderto generate error page content but doesn't dispose it:Since
PhysicalFileProviderimplementsIDisposableand manages file system resources, this could lead to handle leaks.Solution
Wrapped the
PhysicalFileProvidercreation in ausingstatement to ensure proper disposal:This approach follows existing patterns in the codebase where
PhysicalFileProvideris used withusingstatements in similar scenarios (e.g., in diagnostic and static file middleware tests).The fix is safe because
ErrorPageModelBuilder.CreateErrorPageModel()only uses the file provider during its execution and doesn't retain any references to it.Fixes #59524.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.